Skip to content

feat(onboarding): add email verification code UI#3052

Merged
baktun14 merged 4 commits intomainfrom
feat/auth0-verification-frontend
Apr 21, 2026
Merged

feat(onboarding): add email verification code UI#3052
baktun14 merged 4 commits intomainfrom
feat/auth0-verification-frontend

Conversation

@baktun14
Copy link
Copy Markdown
Contributor

@baktun14 baktun14 commented Apr 9, 2026

Why

Part of CON-197 — Replace Auth0's default email verification link with a 6-digit code flow.

This is PR 2 of 2 — frontend only. Split from #2824 to make review manageable.
Depends on PR 1 (backend): #3051

What

  • VerificationCodeInput: 6-digit OTP input with autofill support (extracted per review feedback)
  • EmailVerificationStep: code entry UI with cooldown timer and resend
  • EmailVerificationContainer: lean container orchestrating send/verify/resend (UI concerns moved to view per review feedback)
  • VerifyEmailPage: updated to use new code-based verification
  • SessionService: simplified to remove legacy verification logic
  • useEmailVerificationRequiredEventHandler: updated for code-based flow
  • Snackbar: fix DOM nesting warning
  • Unit tests for all new and modified components

13 files changed (~670 additions, ~465 deletions)

Summary by CodeRabbit

  • New Features

    • New 6-digit verification input with autofill/paste support, resend-code button with 60s cooldown, and explicit send/verify actions.
    • Verification auto-advances when confirmed and onboarding now shows a single redirect/loading state.
  • Bug Fixes

    • Clarified signup and verification error messages and improved success/error toast behavior.
  • Other

    • Analytics event and snackbar subtitle text updated.

@baktun14 baktun14 requested a review from a team as a code owner April 9, 2026 23:08
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 9, 2026

📝 Walkthrough

Walkthrough

Refactors onboarding email verification to an async send/verify API, introduces a 6-digit verification input component with cooldown, simplifies VerifyEmailPage to immediate redirect, moves signup to console API, adjusts analytics/snackbar/event names, and updates tests accordingly. (49 words)

Changes

Cohort / File(s) Summary
Email Verification Container
apps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.tsx, apps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.spec.tsx
Replaced boolean/handler children API with { sendCode(): Promise<void>, verifyCode(code: string): Promise<void> }; removed snackbar/notificator state and resend/check UI; added auto-advance when user.emailVerified and analytics tracking. Tests updated for new API, delegation, and error propagation.
Email Verification Step & Input
apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.tsx, .../EmailVerificationStep.spec.tsx, apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/VerificationCodeInput.tsx, .../VerificationCodeInput.spec.tsx
Added VerificationCodeInput (6-digit inputs, paste/OTP, ref API). EmailVerificationStep now consumes sendCode/verifyCode, manages resend cooldown (60s), local loading, and notificator via DI. New tests cover UI, resend, verify flows and error handling.
Verify Email Page
apps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.tsx, apps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.spec.tsx
Removed query-param-driven verification, useVerifyEmail/useWhen logic and verification UI. Page now calls injected redirect(url) (stores step and uses window.location.replace) inside useEffect and renders a single redirect/loading state. Tests assert redirect behavior.
Onboarding View Integration
apps/deploy-web/src/components/onboarding/OnboardingView/OnboardingView.tsx
Changed child render usage to explicitly extract and pass sendCode and verifyCode from container instead of spreading all container props.
Email Verification Required Handler
apps/deploy-web/src/hooks/useEmailVerificationRequiredEventHandler.tsx
Renamed action/button copy to “Send verification code”, switched from auth.sendVerificationEmail(user.id) to auth.sendVerificationCode(), updated analytics event name and success/error messages; adjusted callback deps.
Signup / Session Service
apps/deploy-web/src/services/session/session.service.ts, apps/deploy-web/src/services/session/session.service.spec.ts
Sign-up moved to console API POST /v1/auth/signup via consoleApiHttpClient; removed Auth0-specific fields and invalid_password error variant; treats existing-user as HTTP 422. Tests updated to reflect new clients, responses, and expectations.
Analytics Event Type
apps/deploy-web/src/services/analytics/analytics.service.ts
Updated AnalyticsEvent union: replaced "resend_verification_email_btn_clk" with "send_verification_code_btn_clk".
Snackbar Component
packages/ui/components/custom/snackbar.tsx
Removed children from Props type and changed subtitle element from <p> to <div>, altering markup semantics.
API: Email Verification Code
apps/api/src/auth/repositories/email-verification-code/..., apps/api/src/auth/services/email-verification-code/...
Removed findByUserIdForUpdate(...) and transaction-wrapped verifyCodeInTransaction; switched to findByUserId(...), perform Auth0 mark-email-verified call directly in verifyCode(). Adjusted cooldown error message text. Tests updated to mock non-locking repo methods.
Session analytics/tests & other test adjustments
apps/deploy-web/..., apps/api/...
Multiple test suites updated or added to reflect API, component, and behavioral changes (new components, modified flows, adjusted error messages and mocks).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • stalniy
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/auth0-verification-frontend

Comment @coderabbitai help to get the list of available commands and usage tips.

Base automatically changed from feat/auth0-verification-backend to main April 20, 2026 18:08
…to-advance

Implement 6-digit code input with OTP autofill, cooldown timer for
resend, auto-advance on verification success, and toast notifications
for errors. Extracts VerificationCodeInput component and simplifies
EmailVerificationContainer/Step separation.
Remove redundant refs (cooldownRef, isSendingRef) that duplicated state,
fix resend button showing "Verifying..." label, remove dead user.id
guard, and inject redirect as dependency to simplify VerifyEmailPage tests.
@baktun14 baktun14 force-pushed the feat/auth0-verification-frontend branch from 126318b to 19eaf9e Compare April 20, 2026 21:09
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

❌ Patch coverage is 83.45865% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.23%. Comparing base (50bf139) to head (450241b).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ps/EmailVerificationStep/VerificationCodeInput.tsx 78.46% 10 Missing and 4 partials ⚠️
...nts/onboarding/VerifyEmailPage/VerifyEmailPage.tsx 40.00% 3 Missing ⚠️
...ps/EmailVerificationStep/EmailVerificationStep.tsx 94.28% 2 Missing ⚠️
...hooks/useEmailVerificationRequiredEventHandler.tsx 0.00% 2 Missing ⚠️
...nents/onboarding/OnboardingView/OnboardingView.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3052      +/-   ##
==========================================
- Coverage   61.05%   60.23%   -0.83%     
==========================================
  Files        1027      988      -39     
  Lines       24593    23647     -946     
  Branches     6063     5907     -156     
==========================================
- Hits        15016    14244     -772     
+ Misses       8363     8200     -163     
+ Partials     1214     1203      -11     
Flag Coverage Δ *Carryforward flag
api 82.69% <100.00%> (-0.01%) ⬇️
deploy-web 45.04% <82.53%> (+0.18%) ⬆️
log-collector ?
notifications 86.06% <ø> (ø) Carriedforward from 5ce1ff0
provider-console 81.48% <ø> (ø)
provider-proxy 85.21% <ø> (ø) Carriedforward from 5ce1ff0
tx-signer ?

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...ication-code/email-verification-code.repository.ts 12.50% <ø> (+1.97%) ⬆️
...rification-code/email-verification-code.service.ts 100.00% <100.00%> (ø)
...rificationContainer/EmailVerificationContainer.tsx 100.00% <100.00%> (ø)
...oy-web/src/services/analytics/analytics.service.ts 94.20% <ø> (ø)
...deploy-web/src/services/session/session.service.ts 78.12% <100.00%> (-1.29%) ⬇️
...nents/onboarding/OnboardingView/OnboardingView.tsx 0.00% <0.00%> (ø)
...ps/EmailVerificationStep/EmailVerificationStep.tsx 94.44% <94.28%> (+94.44%) ⬆️
...hooks/useEmailVerificationRequiredEventHandler.tsx 5.26% <0.00%> (+0.50%) ⬆️
...nts/onboarding/VerifyEmailPage/VerifyEmailPage.tsx 62.50% <40.00%> (-26.39%) ⬇️
...ps/EmailVerificationStep/VerificationCodeInput.tsx 78.46% <78.46%> (ø)

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (2)
apps/deploy-web/src/services/session/session.service.spec.ts (1)

144-170: Make duplicate-user test resilient to backend status mapping.

Line [145] hardcodes 409, but this flow is contract-sensitive. Consider covering both 409 and 422 to prevent brittle behavior across backend versions.

Suggested test hardening
-    it("returns user_exists when user already exists and sign-in fails", async () => {
-      const { service, consoleApiHttpClient, externalHttpClient } = setup();
-
-      consoleApiHttpClient.post.mockResolvedValueOnce({
-        status: 409,
+    it.each([409, 422])("returns user_exists when user already exists (status %s) and sign-in fails", async duplicateStatus => {
+      const { service, consoleApiHttpClient, externalHttpClient } = setup();
+
+      consoleApiHttpClient.post.mockResolvedValueOnce({
+        status: duplicateStatus,
         data: {
           message: "The user already exists."
         },
         headers: {}
       });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/deploy-web/src/services/session/session.service.spec.ts` around lines
144 - 170, The duplicate-user test is brittle because it hardcodes
consoleApiHttpClient.post to return status 409; update the test around
service.signUp to accept the backend mapping change by mocking
consoleApiHttpClient.post to return either 409 or 422 (e.g., parametrize or set
up two sub-cases) while keeping externalHttpClient.post mocked as shown; ensure
the assertion on the resulting error (message "Such user already exists but
credentials are invalid" and code "user_exists") and the call count assertions
for consoleApiHttpClient.post and externalHttpClient.post remain unchanged so
the behavior is validated for both backend status variants.
apps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.spec.tsx (1)

10-13: Prefer getByText for this presence check.

queryByText(...).toBeInTheDocument() makes failures harder to debug because you lose the thrown query error and DOM snapshot.

Based on learnings: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use getBy methods instead of queryBy methods when testing element presence with toBeInTheDocument() because getBy throws an error and shows DOM state when element is not found, providing better debugging information than queryBy which returns null.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.spec.tsx`
around lines 10 - 13, Change the presence assertion in the "shows redirect
loading text" test to use screen.getByText instead of screen.queryByText so
failures throw with DOM snapshot; locate the test that calls setup() in
VerifyEmailPage.spec.tsx and update the expectation referencing the "Redirecting
to email verification..." string to use getByText(...).toBeInTheDocument() (test
helper: setup).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.tsx`:
- Around line 63-70: The reset of the verification input is being called while
isVerifying is still true so the VerificationCodeInput.reset() cannot focus
(disabled inputs ignore focus); change the finally/error handling in the async
flow around verifyCode so that setIsVerifying(false) runs before calling
codeInputRef.current?.reset(), and ensure the reset/refocus is deferred until
after the disabled state clears (e.g., call reset inside a next-tick/setTimeout
0 or after awaiting state flush) so the input can receive focus; update the code
paths around verifyCode, setIsVerifying, and codeInputRef.reset to reflect this
ordering.

In
`@apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/VerificationCodeInput.tsx`:
- Around line 49-77: The multi-character branch in handleDigitChange currently
treats any value.length > 1 as a paste/autofill and scatters characters into
subsequent cells, which causes a typed replacement like "oldChar+newChar" to
spill instead of replacing the current box; update handleDigitChange so that
when value.length > 1 you first check the current stored digit at that index
(use digits or setDigits(prev => ...) to read prev[index]) and if that slot was
already populated and value.length === 2 treat it as a replacement: write the
last character of value into newDigits[index] (not shifting the old one forward)
and only spread any remaining characters after that; otherwise (empty slot or
true paste/autofill of multiple chars) keep the existing
fill-into-following-cells behavior. Apply the same fix to the duplicate logic in
the other identical handler (the block noted around the other occurrence).

In
`@apps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.tsx`:
- Around line 14-16: The redirect implementation in the redirect function
currently sets window.location.href which adds a history entry and causes a
back-button loop for the transient VerifyEmailPage; change it to use
location.replace(url) so the /verify-email step is not kept in history. Update
the redirect function that writes ONBOARDING_STEP_KEY and currently calls
window.location.href to call window.location.replace(url) (preserving the
existing localStorage write to OnboardingStepIndex.EMAIL_VERIFICATION).

In `@apps/deploy-web/src/hooks/useEmailVerificationRequiredEventHandler.tsx`:
- Around line 24-31: The analytics event name
"resend_verification_email_btn_clk" is now incorrect for the button that calls
auth.sendVerificationCode(); update the event emitted by analyticsService.track
in the onClick handler to a new, distinct name (e.g.,
"send_verification_code_btn_clk" or similar) so this flow is not merged with the
old resend metric; locate the analyticsService.track call in
useEmailVerificationRequiredEventHandler (the onClick handler that calls
auth.sendVerificationCode) and replace the string, and update any related tests
or analytics mapping entries that expect the old event name.

In `@apps/deploy-web/src/services/session/session.service.ts`:
- Around line 96-99: The duplicate-user detection in session.service.ts is using
status 409 but the backend maps Auth0's duplicate-user to 422, so update the
check (the isUserExists assignment that currently compares signupResponse.status
=== 409) to treat 422 as the duplicate case (e.g., compare against 422 or
include both 409 and 422), so that the existing branch (return Err when status
>=400 and !isUserExists) and the idempotent fallback in password-signup.ts are
triggered correctly; locate the signupResponse handling in the signup function
in session.service.ts and adjust the status check accordingly.

---

Nitpick comments:
In
`@apps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.spec.tsx`:
- Around line 10-13: Change the presence assertion in the "shows redirect
loading text" test to use screen.getByText instead of screen.queryByText so
failures throw with DOM snapshot; locate the test that calls setup() in
VerifyEmailPage.spec.tsx and update the expectation referencing the "Redirecting
to email verification..." string to use getByText(...).toBeInTheDocument() (test
helper: setup).

In `@apps/deploy-web/src/services/session/session.service.spec.ts`:
- Around line 144-170: The duplicate-user test is brittle because it hardcodes
consoleApiHttpClient.post to return status 409; update the test around
service.signUp to accept the backend mapping change by mocking
consoleApiHttpClient.post to return either 409 or 422 (e.g., parametrize or set
up two sub-cases) while keeping externalHttpClient.post mocked as shown; ensure
the assertion on the resulting error (message "Such user already exists but
credentials are invalid" and code "user_exists") and the call count assertions
for consoleApiHttpClient.post and externalHttpClient.post remain unchanged so
the behavior is validated for both backend status variants.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b6b50647-33f9-4d31-8564-eab3c0f9772f

📥 Commits

Reviewing files that changed from the base of the PR and between 50bf139 and 19eaf9e.

📒 Files selected for processing (13)
  • apps/deploy-web/src/components/onboarding/OnboardingView/OnboardingView.tsx
  • apps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.spec.tsx
  • apps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.tsx
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.spec.tsx
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.tsx
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.spec.tsx
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.tsx
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/VerificationCodeInput.spec.tsx
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/VerificationCodeInput.tsx
  • apps/deploy-web/src/hooks/useEmailVerificationRequiredEventHandler.tsx
  • apps/deploy-web/src/services/session/session.service.spec.ts
  • apps/deploy-web/src/services/session/session.service.ts
  • packages/ui/components/custom/snackbar.tsx

Comment thread apps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.tsx Outdated
Comment thread apps/deploy-web/src/services/session/session.service.ts Outdated
- use location.replace on /verify-email to avoid back-button loop
- defer code input reset so focus lands after input is re-enabled
- select digit on focus so re-typing replaces instead of spilling
- rename analytics event to send_verification_code_btn_clk
- detect duplicate signup via 422 (matches auth.controller mapping)
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
apps/deploy-web/src/services/session/session.service.spec.ts (1)

168-169: Strengthen signup-path assertions to avoid false positives.

These checks are currently too loose for a critical auth flow: the duplicate-user case only asserts call counts, and the token URL assertion accepts any host as long as "/oauth/token" is present. Please assert endpoint + payload explicitly in both scenarios.

Proposed test hardening
-      const { service, externalHttpClient, consoleApiHttpClient } = setup();
+      const { service, externalHttpClient, consoleApiHttpClient, config } = setup();

@@
-      expect(consoleApiHttpClient.post).toHaveBeenCalledTimes(1);
-      expect(externalHttpClient.post).toHaveBeenCalledTimes(1);
+      expect(consoleApiHttpClient.post).toHaveBeenNthCalledWith(
+        1,
+        "/v1/auth/signup",
+        { email: "user@example.com", password: "Password123!" },
+        expect.objectContaining({ validateStatus: expect.any(Function) })
+      );
+      expect(externalHttpClient.post).toHaveBeenNthCalledWith(
+        1,
+        `${new URL(config.ISSUER_BASE_URL).origin}/oauth/token`,
+        expect.objectContaining({
+          username: "user@example.com",
+          password: "Password123!"
+        }),
+        expect.objectContaining({ validateStatus: expect.any(Function) })
+      );

@@
-      expect(externalHttpClient.post).toHaveBeenNthCalledWith(
-        1,
-        expect.stringContaining("/oauth/token"),
+      expect(externalHttpClient.post).toHaveBeenNthCalledWith(
+        1,
+        `${new URL(config.ISSUER_BASE_URL).origin}/oauth/token`,
         expect.objectContaining({
           username: email,
           password
         }),
         expect.objectContaining({ validateStatus: expect.any(Function) })
       );

As per coding guidelines: "Verify meaningful assertions, not just snapshot coverage."

Also applies to: 235-237

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/deploy-web/src/services/session/session.service.spec.ts` around lines
168 - 169, The test currently only checks call counts for
consoleApiHttpClient.post and externalHttpClient.post and loosely checks the
token URL; strengthen the assertions by verifying the exact endpoint and request
payloads using toHaveBeenCalledWith (e.g., assert consoleApiHttpClient.post was
called with the specific signup/duplicate-user endpoint and body, and assert
externalHttpClient.post was called with the full token URL including host and
the exact token request payload/headers). Update both the duplicate-user
assertions around consoleApiHttpClient.post and the token assertions (lines
referenced near 168 and also 235-237) to explicitly check method arguments
rather than just presence of "/oauth/token" or call counts.
apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.tsx (1)

44-57: Reset on line 46 may clear user input prematurely.

codeInputRef.current?.reset() is called at the start of handleResendCode, before sendCode() is awaited. If the API call fails, the user loses their partially-typed code. Consider moving the reset to the success path only.

♻️ Proposed fix
   const handleResendCode = useCallback(async () => {
     setIsResending(true);
-    codeInputRef.current?.reset();

     try {
       await sendCode();
       setCooldownSeconds(COOLDOWN_DURATION);
+      codeInputRef.current?.reset();
       notificator.success("Verification code sent. Please check your email for the 6-digit code.");
     } catch {
       notificator.error("Failed to send verification code. Please try again later");
     } finally {
       setIsResending(false);
     }
   }, [sendCode, notificator]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.tsx`
around lines 44 - 57, In handleResendCode, avoid clearing the user's
partially-typed input up-front: move the call to codeInputRef.current?.reset()
out of the start of the function and into the success branch after await
sendCode() succeeds; keep setIsResending(true) at the top and the final
setIsResending(false) in finally, and still call
setCooldownSeconds(COOLDOWN_DURATION) and notificator.success on success, while
notificator.error remains in the catch branch so the input is only reset when
sendCode() succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.tsx`:
- Around line 44-57: In handleResendCode, avoid clearing the user's
partially-typed input up-front: move the call to codeInputRef.current?.reset()
out of the start of the function and into the success branch after await
sendCode() succeeds; keep setIsResending(true) at the top and the final
setIsResending(false) in finally, and still call
setCooldownSeconds(COOLDOWN_DURATION) and notificator.success on success, while
notificator.error remains in the catch branch so the input is only reset when
sendCode() succeeds.

In `@apps/deploy-web/src/services/session/session.service.spec.ts`:
- Around line 168-169: The test currently only checks call counts for
consoleApiHttpClient.post and externalHttpClient.post and loosely checks the
token URL; strengthen the assertions by verifying the exact endpoint and request
payloads using toHaveBeenCalledWith (e.g., assert consoleApiHttpClient.post was
called with the specific signup/duplicate-user endpoint and body, and assert
externalHttpClient.post was called with the full token URL including host and
the exact token request payload/headers). Update both the duplicate-user
assertions around consoleApiHttpClient.post and the token assertions (lines
referenced near 168 and also 235-237) to explicitly check method arguments
rather than just presence of "/oauth/token" or call counts.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fcd0834b-0ede-41e9-bf8b-9d88d395f70b

📥 Commits

Reviewing files that changed from the base of the PR and between 19eaf9e and 5ce1ff0.

📒 Files selected for processing (7)
  • apps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.tsx
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.tsx
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/VerificationCodeInput.tsx
  • apps/deploy-web/src/hooks/useEmailVerificationRequiredEventHandler.tsx
  • apps/deploy-web/src/services/analytics/analytics.service.ts
  • apps/deploy-web/src/services/session/session.service.spec.ts
  • apps/deploy-web/src/services/session/session.service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/VerificationCodeInput.tsx
  • apps/deploy-web/src/services/session/session.service.ts

Wrapping verifyCode in a transaction caused the attempts increment to
be rolled back by the subsequent 400 throw, letting clients retry
indefinitely. Drop the transaction (no multi-row invariant needs it),
clarify the cooldown message, and surface backend errors on the resend
path instead of a generic toast.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/api/src/auth/services/email-verification-code/email-verification-code.service.ts (1)

64-75: ⚠️ Potential issue | 🔴 Critical

Race condition allows concurrent code guessing attempts to bypass MAX_ATTEMPTS limit.

Removing the transaction introduced a TOCTOU vulnerability. Multiple concurrent verifyCode requests can each read the same attempts count (e.g., 0), all pass the MAX_ATTEMPTS check, and try different codes. With 5 concurrent requests, an attacker effectively gets 5 guesses per round instead of 5 total, making brute-forcing a 6-digit code feasible.

The commit fixed the previous issue (transaction rolling back increments on 400 errors) but created this new one. Consider:

  1. Add SELECT ... FOR UPDATE to serialize verification attempts per user, or
  2. Use atomic UPDATE ... WHERE attempts < MAX_ATTEMPTS RETURNING * to conditionally increment, or
  3. Implement Redis-based rate limiting per userId.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/api/src/auth/services/email-verification-code/email-verification-code.service.ts`
around lines 64 - 75, verifyCode currently reads the verification record and
then calls emailVerificationCodeRepository.incrementAttempts, which opens a
TOCTOU race allowing concurrent guesses to bypass MAX_ATTEMPTS; fix by making
the attempts check+increment atomic inside the repository or by locking the row
before checking: either implement an atomic method like
emailVerificationCodeRepository.incrementAttemptsIfUnderLimit(record.id,
MAX_ATTEMPTS) that performs a conditional UPDATE ... WHERE attempts <
MAX_ATTEMPTS RETURNING the row (and fail if no row returned), or wrap the
read/check/update in a DB transaction using SELECT ... FOR UPDATE on the row
returned by findByUserId so verifyCode enforces the attempts check serially;
update verifyCode to use the new repository method or the transactional flow and
to treat a failed conditional update as exceeding MAX_ATTEMPTS.
🧹 Nitpick comments (1)
apps/api/src/auth/services/email-verification-code/email-verification-code.service.ts (1)

77-84: Local DB updated before Auth0 call—intentional but creates inconsistency window.

If markEmailVerified fails (line 80), the local DB already has emailVerified: true but Auth0 does not. The test at line 165 confirms this is intentional. However, this means a user could be locally verified but Auth0-unverified, which may cause issues if downstream code relies on Auth0's verification status.

Consider adding a reconciliation mechanism or retry queue for failed Auth0 updates if this mismatch causes problems in production.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/api/src/auth/services/email-verification-code/email-verification-code.service.ts`
around lines 77 - 84, The code updates the local flag via
userRepository.updateById before calling auth0Service.markEmailVerified,
creating a potential inconsistency if markEmailVerified fails; modify
email-verification-code.service to handle failures by either (a) rolling back
the local change when markEmailVerified throws (call
userRepository.updateById(userInternalId, { emailVerified: false }) in the
catch) or (b) enqueueing a retry job (push a job to your existing
retry/reconciliation queue with userInternalId and auth0UserId) and keep the
local change, and ensure logger.error still records the failure; pick one
approach and implement it around the auth0Service.markEmailVerified call and its
catch block so the system either reverts local state or reliably retries the
Auth0 update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In
`@apps/api/src/auth/services/email-verification-code/email-verification-code.service.ts`:
- Around line 64-75: verifyCode currently reads the verification record and then
calls emailVerificationCodeRepository.incrementAttempts, which opens a TOCTOU
race allowing concurrent guesses to bypass MAX_ATTEMPTS; fix by making the
attempts check+increment atomic inside the repository or by locking the row
before checking: either implement an atomic method like
emailVerificationCodeRepository.incrementAttemptsIfUnderLimit(record.id,
MAX_ATTEMPTS) that performs a conditional UPDATE ... WHERE attempts <
MAX_ATTEMPTS RETURNING the row (and fail if no row returned), or wrap the
read/check/update in a DB transaction using SELECT ... FOR UPDATE on the row
returned by findByUserId so verifyCode enforces the attempts check serially;
update verifyCode to use the new repository method or the transactional flow and
to treat a failed conditional update as exceeding MAX_ATTEMPTS.

---

Nitpick comments:
In
`@apps/api/src/auth/services/email-verification-code/email-verification-code.service.ts`:
- Around line 77-84: The code updates the local flag via
userRepository.updateById before calling auth0Service.markEmailVerified,
creating a potential inconsistency if markEmailVerified fails; modify
email-verification-code.service to handle failures by either (a) rolling back
the local change when markEmailVerified throws (call
userRepository.updateById(userInternalId, { emailVerified: false }) in the
catch) or (b) enqueueing a retry job (push a job to your existing
retry/reconciliation queue with userInternalId and auth0UserId) and keep the
local change, and ensure logger.error still records the failure; pick one
approach and implement it around the auth0Service.markEmailVerified call and its
catch block so the system either reverts local state or reliably retries the
Auth0 update.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5938ef4d-69b4-40d8-80c3-32e17a9069e4

📥 Commits

Reviewing files that changed from the base of the PR and between 5ce1ff0 and 450241b.

📒 Files selected for processing (5)
  • apps/api/src/auth/repositories/email-verification-code/email-verification-code.repository.ts
  • apps/api/src/auth/services/email-verification-code/email-verification-code.service.spec.ts
  • apps/api/src/auth/services/email-verification-code/email-verification-code.service.ts
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.spec.tsx
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.tsx
💤 Files with no reviewable changes (1)
  • apps/api/src/auth/repositories/email-verification-code/email-verification-code.repository.ts
✅ Files skipped from review due to trivial changes (1)
  • apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.spec.tsx

@baktun14 baktun14 added this pull request to the merge queue Apr 21, 2026
Merged via the queue into main with commit d178698 Apr 21, 2026
53 checks passed
@baktun14 baktun14 deleted the feat/auth0-verification-frontend branch April 21, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants